-
-
Notifications
You must be signed in to change notification settings - Fork 10k
[Feature][Response API] Add streaming support for non-harmony #23741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature][Response API] Add streaming support for non-harmony #23741
Conversation
8dc2da4
to
b65638e
Compare
b65638e
to
3bb6902
Compare
6d9fe9c
to
af25d9a
Compare
@heheda12345 PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution. Some small comments.
) -> AsyncGenerator[str, None]: | ||
sequence_number = 0 | ||
current_content_index = 0 # FIXME: this number is never changed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you fix these indexes? Reference: #23382
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
77bd0aa
to
bc4c5ae
Compare
@@ -864,7 +861,7 @@ async def _process_simple_streaming_events( | |||
created_time: int, | |||
_send_event: Callable[[BaseModel], str], | |||
) -> AsyncGenerator[str, None]: | |||
current_content_index = 0 # FIXME: this number is never changed | |||
current_content_index = 0 | |||
current_output_index = 0 | |||
current_item_id = "" # FIXME: this number is never changed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick update. Can you also update the "current_item_id"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the reminder, my apologies for the oversight, fixed.
bc4c5ae
to
cf993d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for your contribution.
@kebe7jun The v1-test-entrypoints CI failure seems to be related to this PR. Can you take a look?
|
Head branch was pushed to by a user without write access
77ef2de
to
30d435e
Compare
Signed-off-by: Kebe <[email protected]>
Signed-off-by: Kebe <[email protected]>
Signed-off-by: Kebe <[email protected]>
30d435e
to
3e604da
Compare
…roject#23741) Signed-off-by: Kebe <[email protected]> Signed-off-by: JasonZhu1313 <[email protected]>
Purpose
Add streaming support for non-harmony
Related issue #23225
Test Plan
Unit tests and self tests(see result).
Test Result
GPT-OSS Stream output
Qwen3 30B A3B Stream output
Essential Elements of an Effective PR Description Checklist
supported_models.md
andexamples
for a new model.